[PM-8214] New Device Verification Notice UI #12360

Jingo88 · 2024-12-11T23:17:17Z

🎟️ Tracking

📔 Objective

Create the components and design for all three clients.

Translation keys added
New svg's added
New Route Guard added to vault routes
New guard will check for NewDeviceVerification feature flags and state values
Clicking on two step login or account email buttons will navigate to those settings pages
Clicking through the flow will navigate to vault and update state

NOTE: This PR implements the designs, full guard and state retention logic will be added afterwards here PM-14347

📸 Screenshots

Screen Recording of Web Flow
https://github.com/user-attachments/assets/fa4074f0-debc-4aae-9411-12550d311f3b

Screen Recording of Browser Flow
https://github.com/user-attachments/assets/c643c02c-56c4-4e82-9461-ec8052191b14

Screen Recording of Browser Navigating to Settings
https://github.com/user-attachments/assets/669e18e7-fba0-4041-9647-2b057f39914a

Screenshots of Desktop

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

…lt based on flags, and can navigate back to vault after submission

github-actions · 2024-12-11T23:27:45Z

Checkmarx One – Scan Summary & Details – 65dd07cb-9422-4f10-adb1-5017cfc3461f

New Issues

Severity	Issue	Source File / Package	Checkmarx Insight
	Client_Privacy_Violation	/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.html: 40	Attack Vector
	Client_Privacy_Violation	/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.html: 50	Attack Vector

Fixed Issues

Severity	Issue	Source File / Package
	Client_Privacy_Violation	/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.html: 40
	Client_Privacy_Violation	/bitwarden_license/bit-web/src/app/tools/access-intelligence/password-health.component.html: 50

codecov · 2024-12-12T17:02:52Z

Codecov Report

Attention: Patch coverage is 27.11864% with 86 lines in your changes missing coverage. Please review.

Project coverage is 33.50%. Comparing base (1d874b4) to head (2ac259d).
Report is 18 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ult/guards/new-device-verification-notice.guard.ts	0.00%	29 Missing ⚠️
...w-device-verification-notice-page-two.component.ts	30.00%	24 Missing and 4 partials ⚠️
...w-device-verification-notice-page-one.component.ts	36.36%	17 Missing and 4 partials ⚠️
apps/browser/src/popup/app-routing.module.ts	0.00%	2 Missing ⚠️
apps/desktop/src/app/app-routing.module.ts	0.00%	2 Missing ⚠️
apps/web/src/app/oss-routing.module.ts	0.00%	2 Missing ⚠️
libs/angular/src/services/jslib-services.module.ts	0.00%	1 Missing ⚠️
libs/angular/src/vault/guards/index.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12360      +/-   ##
==========================================
- Coverage   33.52%   33.50%   -0.02%     
==========================================
  Files        2913     2921       +8     
  Lines       91086    91229     +143     
  Branches    17338    17358      +20     
==========================================
+ Hits        30540    30570      +30     
- Misses      58139    58251     +112     
- Partials     2407     2408       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nick-livefront

Nice! This is working as I'd expect, just a nit and a question.

nick-livefront · 2024-12-13T15:43:53Z

libs/angular/src/vault/guards/new-device-verification-notice.guard.ts

+  if (
+    userItems?.last_dismissal == null &&
+    (userItems?.permanent_dismissal == null || !userItems?.permanent_dismissal) &&
+    (tempNoticeFlag || permNoticeFlag)
+  ) {


⛏️ (non-blocking) this logic could be cleaned up by deconstructing userItems. The types don't suggest that userItems can be null so I removed the optional chaining as well.

Suggested change

if (

userItems?.last_dismissal == null &&

(userItems?.permanent_dismissal == null || !userItems?.permanent_dismissal) &&

(tempNoticeFlag || permNoticeFlag)

) {

const { last_dismissal, permanent_dismissal } = userItems;

if (last_dismissal == null && !permanent_dismissal && (tempNoticeFlag || permNoticeFlag)) {

I have this code altered in my following work so this is does not need to be changed here.

...onents/new-device-verification-notice/new-device-verification-notice-page-one.component.html

nick-livefront · 2024-12-16T16:00:48Z

libs/angular/src/utils/new-device-notification-swap.ts

❓ Is this needed?

Removed: 78fd445

…214-new-device-notice-UI

nick-livefront · 2024-12-17T20:40:20Z

libs/vault/src/icons/exclamation-triangle.ts

🎨 Should the icon use tailwind for fill to support light/dark mode?

Updated for both icons: 54d872a

shane-melton

Looks pretty good to me, just two questions before approval (and a few non-blocking nits)

...mponents/new-device-verification-notice/new-device-verification-notice-page-two.component.ts

...onents/new-device-verification-notice/new-device-verification-notice-page-two.component.html

...mponents/new-device-verification-notice/new-device-verification-notice-page-one.component.ts

libs/vault/src/icons/exclamation-triangle.ts

...mponents/new-device-verification-notice/new-device-verification-notice-page-one.component.ts

- an empty href is needed so the links are keyboard accessible

- No noticeable difference in the end result

Jingo88 added 21 commits November 22, 2024 10:41

starting

e0697a6

Merge branch 'main' into PM-8214-new-device-notice-UI

7fe1614

setup first page for new device verification notice

2affcdf

pulled latest, fixed conflict

d82758a

update designs for first page. rename components and files

d20b7a5

added second page for new device verification notice

64598a0

Merge branch 'main' into PM-8214-new-device-notice-UI

5f41eed

update notice page one with bit radio buttons. routing logic. user email

f96ef7e

pulled latest. fixed conflicts

11396b0

updated routing for new device verification notice to show before vau…

954cfe9

…lt based on flags, and can navigate back to vault after submission

Merge branch 'main' into PM-8214-new-device-notice-UI

f011abb

fix translations. added remind me later link and nav to page 2

ecdff4b

Merge branch 'main' into PM-8214-new-device-notice-UI

4c335a2

sync the design for mobile and web

fa70982

Merge branch 'main' into PM-8214-new-device-notice-UI

86b8e87

Merge branch 'main' into PM-8214-new-device-notice-UI

49dbc18

update routes in desktop

9f4514d

Merge branch 'main' into PM-8214-new-device-notice-UI

4880080

updated styles for desktop

5c25444

Merge branch 'main' into PM-8214-new-device-notice-UI

10a8bae

moved new device verification notice guard

407b5ca

Jingo88 requested a review from a team as a code owner December 11, 2024 23:17

Jingo88 requested review from gbubemismith, shane-melton, cd-bitwarden and nick-livefront December 11, 2024 23:17

Jingo88 added 3 commits December 11, 2024 18:54

update types for new device notice page one

83421c4

add null check to page one

ac87737

types

7bf186c

Jingo88 added 3 commits December 11, 2024 19:25

types for page one, page two, service, and guard

4b1a2e9

types

6e6228a

update component and guard for null check

847911c

add navigation to two step login btn and account email btn

e441795

nick-livefront previously approved these changes Dec 13, 2024

View reviewed changes

nick-livefront reviewed Dec 16, 2024

View reviewed changes

nick-livefront added 2 commits December 16, 2024 21:22

Merge branch 'main' of https://github.com/bitwarden/clients into PM-8…

e95377d

…214-new-device-notice-UI

remove empty file

78fd445

nick-livefront dismissed their stale review via 78fd445 December 17, 2024 03:24

Merge branch 'main' of https://github.com/bitwarden/clients into PM-8…

37fb0ac

…214-new-device-notice-UI

nick-livefront reviewed Dec 17, 2024

View reviewed changes

update fill of icons to support light & dark modes

54d872a

nick-livefront mentioned this pull request Dec 17, 2024

[PM-14347][PM-14348] New Device Verification Logic #12451

Merged

nick-livefront previously approved these changes Dec 17, 2024

View reviewed changes

add question mark to email access verification copy

659e2a7

nick-livefront dismissed their stale review via 659e2a7 December 17, 2024 21:12

shane-melton reviewed Dec 17, 2024

View reviewed changes

nick-livefront added 4 commits December 18, 2024 10:24

remove unused map

84fb842

use links for navigation elements

e3c74cd

- an empty href is needed so the links are keyboard accessible

remove clip path from exclamation svg

072c58f

- No noticeable difference in the end result

inline email message into markup

2ac259d

nick-livefront requested a review from shane-melton December 18, 2024 17:28

shane-melton approved these changes Dec 18, 2024

View reviewed changes

nick-livefront merged commit 1d04a0a into main Dec 19, 2024
73 checks passed

nick-livefront deleted the PM-8214-new-device-notice-UI branch December 19, 2024 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM-8214] New Device Verification Notice UI #12360

[PM-8214] New Device Verification Notice UI #12360

Jingo88 commented Dec 11, 2024 •

edited

Loading

github-actions bot commented Dec 11, 2024 •

edited

Loading

codecov bot commented Dec 12, 2024 •

edited

Loading

nick-livefront left a comment

nick-livefront Dec 13, 2024

nick-livefront Dec 17, 2024

nick-livefront Dec 16, 2024

nick-livefront Dec 17, 2024

nick-livefront Dec 17, 2024

nick-livefront Dec 17, 2024

shane-melton left a comment

[PM-8214] New Device Verification Notice UI #12360

[PM-8214] New Device Verification Notice UI #12360

Conversation

Jingo88 commented Dec 11, 2024 • edited Loading

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

github-actions bot commented Dec 11, 2024 • edited Loading

New Issues

Fixed Issues

codecov bot commented Dec 12, 2024 • edited Loading

Codecov Report

nick-livefront left a comment

Choose a reason for hiding this comment

nick-livefront Dec 13, 2024

Choose a reason for hiding this comment

nick-livefront Dec 17, 2024

Choose a reason for hiding this comment

nick-livefront Dec 16, 2024

Choose a reason for hiding this comment

nick-livefront Dec 17, 2024

Choose a reason for hiding this comment

nick-livefront Dec 17, 2024

Choose a reason for hiding this comment

nick-livefront Dec 17, 2024

Choose a reason for hiding this comment

shane-melton left a comment

Choose a reason for hiding this comment

Jingo88 commented Dec 11, 2024 •

edited

Loading

github-actions bot commented Dec 11, 2024 •

edited

Loading

codecov bot commented Dec 12, 2024 •

edited

Loading